-
Notifications
You must be signed in to change notification settings - Fork 162
feat: add xml parser guidance hook #999
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR introduces a guidance hook for XML parsers to help detect SSRF vulnerabilities by steering fuzzer inputs towards patterns that trigger external entity resolution. The hook guides inputs to contain DOCTYPE declarations with external system identifiers, allowing existing SSRF detection mechanisms to catch unauthorized network connections.
Key Changes
- Added
XmlParserSsrfGuidancehook that intercepts XML parser entry points and guides inputs toward patterns with external DOCTYPE declarations - Refactored
Utils.ktto extract reusable stream peeking utilities (peekMarkableInputStream,peekMarkableReader) - Added
SsrfXmlParsertest case to verify the guidance hook triggers SSRF findings
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| sanitizers/src/main/java/com/code_intelligence/jazzer/sanitizers/XmlParserSsrfGuidance.kt | Core guidance hook implementation that intercepts XML parser methods and guides inputs toward external entity patterns |
| sanitizers/src/main/java/com/code_intelligence/jazzer/sanitizers/Utils.kt | Refactored to provide reusable peekMarkableInputStream and peekMarkableReader utilities |
| sanitizers/src/main/java/com/code_intelligence/jazzer/sanitizers/Deserialization.kt | Updated to use refactored peekMarkableInputStream utility |
| sanitizers/src/test/java/com/example/SsrfXmlParser.java | Test case with vulnerable XML parser configuration |
| sanitizers/src/test/java/com/example/BUILD.bazel | Build configuration for the new test |
| sanitizers/src/main/java/com/code_intelligence/jazzer/sanitizers/BUILD.bazel | Added XmlParserSsrfGuidance to build targets |
| sanitizers/sanitizers.bzl | Registered XmlParserSsrfGuidance in sanitizer class list |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
sanitizers/src/main/java/com/code_intelligence/jazzer/sanitizers/XmlParserSsrfGuidance.kt
Outdated
Show resolved
Hide resolved
sanitizers/src/main/java/com/code_intelligence/jazzer/sanitizers/Utils.kt
Outdated
Show resolved
Hide resolved
45b99a9 to
62c60c9
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice!
62c60c9 to
1f4ac32
Compare
This hook guides the fuzzer to trigger external resource loading during XML parsing, which may trigger a finding reported by the SSRF bug detector.
1f4ac32 to
828c81f
Compare
| n += count | ||
| } | ||
| return current | ||
| return if (n >= readlimit) current else current.copyOf(n) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Really minor: Shouldn't this use size instead of readlimit for consistency, since size is the parameter of this local function (and it just happens to be called with readlimit as value)?
Also, would it make sense to switch the branches? Due to Copilot's suggestion to use >=, this statement looks a bit confusing now. What about this?
| return if (n >= readlimit) current else current.copyOf(n) | |
| return if (n < size) current.copyOf(n) else current |
(Sorry if this comment is distruptive; please let me know in that case)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not disruptive at all. I completely agree. :)
I created a small PR: #1011
A simple guidance hook to nudge Jazzer towards inserting an document xml tag with an external system identifier. If an xml parser is called with fuzzer input and configured to load external entities this will trigger a finding from the SSRF bug detector.